From 4afc5bfb18049c2938961950cadd5987e0a52212 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 9 Dec 2025 23:50:18 +0100 Subject: [PATCH] [PATCH] src: rethrow stack overflow exceptions in async_hooks When a stack overflow exception occurs during async_hooks callbacks (which use TryCatchScope::kFatal), detect the specific "Maximum call stack size exceeded" RangeError and re-throw it instead of immediately calling FatalException. This allows user code to catch the exception with try-catch blocks instead of requiring uncaughtException handlers. The implementation adds IsStackOverflowError() helper to detect stack overflow RangeErrors and re-throws them in TryCatchScope destructor instead of calling FatalException. This fixes the issue where async_hooks would cause stack overflow exceptions to exit with code 7 (kExceptionInFatalExceptionHandler) instead of being catchable. Fixes: https://github.com/nodejs/node/issues/37989 Ref: https://hackerone.com/reports/3456295 PR-URL: https://github.com/nodejs-private/node-private/pull/773 Refs: https://hackerone.com/reports/3456295 Reviewed-By: Robert Nagy Reviewed-By: Paolo Insogna Reviewed-By: Marco Ippolito Reviewed-By: Rafael Gonzaga Reviewed-By: Anna Henningsen CVE-ID: CVE-2025-59466 Gbp-Pq: Topic sec Gbp-Pq: Name 37-rethrow-stack-overflow-exceptions-in-async-hooks.patch --- src/async_wrap.cc | 9 ++- src/debug_utils.cc | 3 +- src/node_errors.cc | 71 ++++++++++++++-- src/node_errors.h | 2 +- src/node_report.cc | 3 +- ...async-hooks-stack-overflow-nested-async.js | 80 +++++++++++++++++++ ...st-async-hooks-stack-overflow-try-catch.js | 47 +++++++++++ .../test-async-hooks-stack-overflow.js | 47 +++++++++++ ...andler-stack-overflow-on-stack-overflow.js | 29 +++++++ ...caught-exception-handler-stack-overflow.js | 29 +++++++ 10 files changed, 306 insertions(+), 14 deletions(-) create mode 100644 test/parallel/test-async-hooks-stack-overflow-nested-async.js create mode 100644 test/parallel/test-async-hooks-stack-overflow-try-catch.js create mode 100644 test/parallel/test-async-hooks-stack-overflow.js create mode 100644 test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js create mode 100644 test/parallel/test-uncaught-exception-handler-stack-overflow.js diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 65829a31a..2b6fa1423 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -67,7 +67,8 @@ static const char* const provider_names[] = { void AsyncWrap::DestroyAsyncIdsCallback(Environment* env) { Local fn = env->async_hooks_destroy_function(); - TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); + TryCatchScope try_catch(env, + TryCatchScope::CatchMode::kFatalRethrowStackOverflow); do { std::vector destroy_async_id_list; @@ -96,7 +97,8 @@ void Emit(Environment* env, double async_id, AsyncHooks::Fields type, HandleScope handle_scope(env->isolate()); Local async_id_value = Number::New(env->isolate(), async_id); - TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); + TryCatchScope try_catch(env, + TryCatchScope::CatchMode::kFatalRethrowStackOverflow); USE(fn->Call(env->context(), Undefined(env->isolate()), 1, &async_id_value)); } @@ -668,7 +670,8 @@ void AsyncWrap::EmitAsyncInit(Environment* env, object, }; - TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); + TryCatchScope try_catch(env, + TryCatchScope::CatchMode::kFatalRethrowStackOverflow); USE(init_fn->Call(env->context(), object, arraysize(argv), argv)); } diff --git a/src/debug_utils.cc b/src/debug_utils.cc index 9d36082db..7d85ab726 100644 --- a/src/debug_utils.cc +++ b/src/debug_utils.cc @@ -333,7 +333,8 @@ void DumpJavaScriptBacktrace(FILE* fp) { } Local stack; - if (!GetCurrentStackTrace(isolate).ToLocal(&stack)) { + if (!GetCurrentStackTrace(isolate).ToLocal(&stack) || + stack->GetFrameCount() == 0) { return; } diff --git a/src/node_errors.cc b/src/node_errors.cc index 52c017984..e458b075a 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -193,7 +193,7 @@ static std::string GetErrorSource(Isolate* isolate, } static std::atomic is_in_oom{false}; -static std::atomic is_retrieving_js_stacktrace{false}; +static thread_local std::atomic is_retrieving_js_stacktrace{false}; MaybeLocal GetCurrentStackTrace(Isolate* isolate, int frame_count) { if (isolate == nullptr) { return MaybeLocal(); @@ -221,9 +221,6 @@ MaybeLocal GetCurrentStackTrace(Isolate* isolate, int frame_count) { StackTrace::CurrentStackTrace(isolate, frame_count, options); is_retrieving_js_stacktrace.store(false); - if (stack->GetFrameCount() == 0) { - return MaybeLocal(); - } return scope.Escape(stack); } @@ -298,7 +295,8 @@ void PrintStackTrace(Isolate* isolate, void PrintCurrentStackTrace(Isolate* isolate, StackTracePrefix prefix) { Local stack; - if (GetCurrentStackTrace(isolate).ToLocal(&stack)) { + if (GetCurrentStackTrace(isolate).ToLocal(&stack) && + stack->GetFrameCount() > 0) { PrintStackTrace(isolate, stack, prefix); } } @@ -669,13 +667,52 @@ v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( }; } +// Check if an exception is a stack overflow error (RangeError with +// "Maximum call stack size exceeded" message). This is used to handle +// stack overflow specially in TryCatchScope - instead of immediately +// exiting, we can use the red zone to re-throw to user code. +static bool IsStackOverflowError(Isolate* isolate, Local exception) { + if (!exception->IsNativeError()) return false; + + Local err_obj = exception.As(); + Local constructor_name = err_obj->GetConstructorName(); + + // Must be a RangeError + Utf8Value name(isolate, constructor_name); + if (name.ToStringView() != "RangeError") return false; + + // Check for the specific stack overflow message + Local context = isolate->GetCurrentContext(); + Local message_val; + if (!err_obj->Get(context, String::NewFromUtf8Literal(isolate, "message")) + .ToLocal(&message_val)) { + return false; + } + + if (!message_val->IsString()) return false; + + Utf8Value message(isolate, message_val.As()); + return message.ToStringView() == "Maximum call stack size exceeded"; +} + namespace errors { TryCatchScope::~TryCatchScope() { - if (HasCaught() && !HasTerminated() && mode_ == CatchMode::kFatal) { + if (HasCaught() && !HasTerminated() && mode_ != CatchMode::kNormal) { HandleScope scope(env_->isolate()); Local exception = Exception(); Local message = Message(); + + // Special handling for stack overflow errors in async_hooks: instead of + // immediately exiting, re-throw the exception. This allows the exception + // to propagate to user code's try-catch blocks. + if (mode_ == CatchMode::kFatalRethrowStackOverflow && + IsStackOverflowError(env_->isolate(), exception)) { + ReThrow(); + Reset(); + return; + } + EnhanceFatalException enhance = CanContinue() ? EnhanceFatalException::kEnhance : EnhanceFatalException::kDontEnhance; if (message.IsEmpty()) @@ -1230,8 +1267,26 @@ void TriggerUncaughtException(Isolate* isolate, if (env->can_call_into_js()) { // We do not expect the global uncaught exception itself to throw any more // exceptions. If it does, exit the current Node.js instance. - errors::TryCatchScope try_catch(env, - errors::TryCatchScope::CatchMode::kFatal); + // Special case: if the original error was a stack overflow and calling + // _fatalException causes another stack overflow, rethrow it to allow + // user code's try-catch blocks to potentially catch it. + auto is_stack_overflow = [&] { + return IsStackOverflowError(env->isolate(), error); + }; + // Without a JS stack, rethrowing may or may not do anything. + // TODO(addaleax): In V8, expose a way to check whether there is a JS stack + // or TryCatch that would capture the rethrown exception. + auto has_js_stack = [&] { + HandleScope handle_scope(env->isolate()); + Local stack; + return GetCurrentStackTrace(env->isolate(), 1).ToLocal(&stack) && + stack->GetFrameCount() > 0; + }; + errors::TryCatchScope::CatchMode mode = + is_stack_overflow() && has_js_stack() + ? errors::TryCatchScope::CatchMode::kFatalRethrowStackOverflow + : errors::TryCatchScope::CatchMode::kFatal; + errors::TryCatchScope try_catch(env, mode); // Explicitly disable verbose exception reporting - // if process._fatalException() throws an error, we don't want it to // trigger the per-isolate message listener which will call this diff --git a/src/node_errors.h b/src/node_errors.h index d5e2f86f5..95ad40abd 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -265,7 +265,7 @@ namespace errors { class TryCatchScope : public v8::TryCatch { public: - enum class CatchMode { kNormal, kFatal }; + enum class CatchMode { kNormal, kFatal, kFatalRethrowStackOverflow }; explicit TryCatchScope(Environment* env, CatchMode mode = CatchMode::kNormal) : v8::TryCatch(env->isolate()), env_(env), mode_(mode) {} diff --git a/src/node_report.cc b/src/node_report.cc index 757f8eb5d..1be665705 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -465,7 +465,8 @@ static void PrintJavaScriptStack(JSONWriter* writer, const char* trigger) { HandleScope scope(isolate); Local stack; - if (!GetCurrentStackTrace(isolate, MAX_FRAME_COUNT).ToLocal(&stack)) { + if (!GetCurrentStackTrace(isolate, MAX_FRAME_COUNT).ToLocal(&stack) || + stack->GetFrameCount() == 0) { PrintEmptyJavaScriptStack(writer); return; } diff --git a/test/parallel/test-async-hooks-stack-overflow-nested-async.js b/test/parallel/test-async-hooks-stack-overflow-nested-async.js new file mode 100644 index 000000000..779f8d75a --- /dev/null +++ b/test/parallel/test-async-hooks-stack-overflow-nested-async.js @@ -0,0 +1,80 @@ +'use strict'; + +// This test verifies that stack overflow during deeply nested async operations +// with async_hooks enabled can be caught by try-catch. This simulates real-world +// scenarios like processing deeply nested JSON structures where each level +// creates async operations (e.g., database calls, API requests). + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + const { createHook } = require('async_hooks'); + + // Enable async_hooks with all callbacks (simulates APM tools) + createHook({ + init() {}, + before() {}, + after() {}, + destroy() {}, + promiseResolve() {}, + }).enable(); + + // Simulate an async operation (like a database call or API request) + async function fetchThing(id) { + return { id, data: `data-${id}` }; + } + + // Recursively process deeply nested data structure + // This will cause stack overflow when the nesting is deep enough + function processData(data, depth = 0) { + if (Array.isArray(data)) { + for (const item of data) { + // Create a promise to trigger async_hooks init callback + fetchThing(depth); + processData(item, depth + 1); + } + } + } + + // Create deeply nested array structure iteratively (to avoid stack overflow + // during creation) + function createNestedArray(depth) { + let result = 'leaf'; + for (let i = 0; i < depth; i++) { + result = [result]; + } + return result; + } + + // Create a very deep nesting that will cause stack overflow during processing + const deeplyNested = createNestedArray(50000); + + try { + processData(deeplyNested); + // Should not complete successfully - the nesting is too deep + console.log('UNEXPECTED: Processing completed without error'); + process.exit(1); + } catch (err) { + assert.strictEqual(err.name, 'RangeError'); + assert.match(err.message, /Maximum call stack size exceeded/); + console.log('SUCCESS: try-catch caught the stack overflow in nested async'); + process.exit(0); + } +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit successfully (try-catch worked) + assert.strictEqual(result.status, 0, + `Expected exit code 0, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); + // Verify the error was handled by try-catch + assert.match(result.stdout, /SUCCESS: try-catch caught the stack overflow/); +} diff --git a/test/parallel/test-async-hooks-stack-overflow-try-catch.js b/test/parallel/test-async-hooks-stack-overflow-try-catch.js new file mode 100644 index 000000000..43338905e --- /dev/null +++ b/test/parallel/test-async-hooks-stack-overflow-try-catch.js @@ -0,0 +1,47 @@ +'use strict'; + +// This test verifies that when a stack overflow occurs with async_hooks +// enabled, the exception can be caught by try-catch blocks in user code. + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + const { createHook } = require('async_hooks'); + + createHook({ init() {} }).enable(); + + function recursive(depth = 0) { + // Create a promise to trigger async_hooks init callback + new Promise(() => {}); + return recursive(depth + 1); + } + + try { + recursive(); + // Should not reach here + process.exit(1); + } catch (err) { + assert.strictEqual(err.name, 'RangeError'); + assert.match(err.message, /Maximum call stack size exceeded/); + console.log('SUCCESS: try-catch caught the stack overflow'); + process.exit(0); + } + + // Should not reach here + process.exit(2); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + assert.strictEqual(result.status, 0, + `Expected exit code 0 (try-catch worked), got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); + assert.match(result.stdout, /SUCCESS: try-catch caught the stack overflow/); +} diff --git a/test/parallel/test-async-hooks-stack-overflow.js b/test/parallel/test-async-hooks-stack-overflow.js new file mode 100644 index 000000000..aff41969d --- /dev/null +++ b/test/parallel/test-async-hooks-stack-overflow.js @@ -0,0 +1,47 @@ +'use strict'; + +// This test verifies that when a stack overflow occurs with async_hooks +// enabled, the uncaughtException handler is still called instead of the +// process crashing with exit code 7. + +const common = require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + const { createHook } = require('async_hooks'); + + let handlerCalled = false; + + function recursive() { + // Create a promise to trigger async_hooks init callback + new Promise(() => {}); + return recursive(); + } + + createHook({ init() {} }).enable(); + + process.on('uncaughtException', common.mustCall((err) => { + assert.strictEqual(err.name, 'RangeError'); + assert.match(err.message, /Maximum call stack size exceeded/); + // Ensure handler is only called once + assert.strictEqual(handlerCalled, false); + handlerCalled = true; + })); + + setImmediate(recursive); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit with code 0 (handler was called and handled the exception) + // Previously would exit with code 7 (kExceptionInFatalExceptionHandler) + assert.strictEqual(result.status, 0, + `Expected exit code 0, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); +} diff --git a/test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js b/test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js new file mode 100644 index 000000000..1923b7f24 --- /dev/null +++ b/test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js @@ -0,0 +1,29 @@ +'use strict'; + +// This test verifies that when the uncaughtException handler itself causes +// a stack overflow, the process exits with a non-zero exit code. +// This is important to ensure we don't silently swallow errors. + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + function f() { f(); } + process.on('uncaughtException', f); + f(); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit with non-zero exit code since the uncaughtException handler + // itself caused a stack overflow. + assert.notStrictEqual(result.status, 0, + `Expected non-zero exit code, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); +} diff --git a/test/parallel/test-uncaught-exception-handler-stack-overflow.js b/test/parallel/test-uncaught-exception-handler-stack-overflow.js new file mode 100644 index 000000000..050cd0923 --- /dev/null +++ b/test/parallel/test-uncaught-exception-handler-stack-overflow.js @@ -0,0 +1,29 @@ +'use strict'; + +// This test verifies that when the uncaughtException handler itself causes +// a stack overflow, the process exits with a non-zero exit code. +// This is important to ensure we don't silently swallow errors. + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + function f() { f(); } + process.on('uncaughtException', f); + throw new Error('X'); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit with non-zero exit code since the uncaughtException handler + // itself caused a stack overflow. + assert.notStrictEqual(result.status, 0, + `Expected non-zero exit code, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); +} -- 2.30.2